Skip to content

Use sync.Pool for mergeIterator batchesBuf - #7765

Open
PaurushGarg wants to merge 1 commit into
cortexproject:masterfrom
PaurushGarg:fix/shared-batches-buf-v2
Open

Use sync.Pool for mergeIterator batchesBuf#7765
PaurushGarg wants to merge 1 commit into
cortexproject:masterfrom
PaurushGarg:fix/shared-batches-buf-v2

Conversation

@PaurushGarg

@PaurushGarg PaurushGarg commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does:
Each mergeIterator allocates its own batchesBuf at creation time. This adds to the heap profile for query engines that create all iterators upfront, especially with high-cardinality queries - while only one buffer is actively used at a time since series are processed sequentially.

This PR replaces per-iterator allocation with a sync.Pool. Each buildNextBatch call borrows a scratch buffer, merges into it, copies the result into c.batches, and returns the buffer via defer. Idle iterators hold no buffer.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

@PaurushGarg
PaurushGarg force-pushed the fix/shared-batches-buf-v2 branch 3 times, most recently from 9706212 to 925fa11 Compare August 17, 2026 19:35
@pull-request-size pull-request-size Bot added size/M and removed size/L labels Aug 17, 2026
@PaurushGarg
PaurushGarg force-pushed the fix/shared-batches-buf-v2 branch 3 times, most recently from 3b7ba40 to 2fdb8d2 Compare August 17, 2026 20:28
@PaurushGarg
PaurushGarg marked this pull request as ready for review August 17, 2026 20:37
@PaurushGarg PaurushGarg changed the title share batchesBuf across merge iterators via MergeWorkspace Use sync.Pool for mergeIterator batchesBuf to reduce memory usage Aug 17, 2026
@PaurushGarg PaurushGarg changed the title Use sync.Pool for mergeIterator batchesBuf to reduce memory usage Use sync.Pool for mergeIterator batchesBuf Aug 17, 2026
@PaurushGarg
PaurushGarg force-pushed the fix/shared-batches-buf-v2 branch from 2fdb8d2 to 32e9cb7 Compare August 17, 2026 20:45
@PaurushGarg
perf(batch): pool batchesBuf in buildNextBatch via sync.Pool
e42270a 
Replace per-iterator batchesBuf with a sync.Pool. Each buildNextBatch
call borrows a scratch buffer from the pool, merges into it, copies the
result into c.batches, and returns the buffer immediately. Idle
iterators hold no buffer at all.

This reduces heap usage from O(series) to O(concurrent_evaluations) —
saving ~1200 bytes per series (~11 GB at 9M series). The pool approach
is engine-agnostic: safe for both sequential and parallel series
processing within a single Select call.

Signed-off-by: Paurush Garg <paurushg@amazon.com>
@PaurushGarg
PaurushGarg force-pushed the fix/shared-batches-buf-v2 branch from 32e9cb7 to e42270a Compare August 18, 2026 16:24
@PaurushGarg

PaurushGarg commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Benchmark results show:
B/op: 93.39 MB → 80.57 MB (-13.72%) — that's the ~13 MB saved for 10K iterators (1300 bytes/series)
But also shows:
create_and_iterate_sequential shows B/op: +9.08% and allocs/op: +112% — because the pool does Get/Put on every buildNextBatch call.

dev-dsk-paurushg-2a-87a5ee42 %   go test ./pkg/querier/batch/ -bench=BenchmarkNewChunkMergeIterator_ManyIterators -run=^$ -benchmem -count=5 | tee new.txt

goos: linux
goarch: amd64
pkg: github.com/cortexproject/cortex/pkg/querier/batch
cpu: AMD EPYC 7R13 Processor
BenchmarkNewChunkMergeIterator_ManyIterators/create_only-32         	      19	  61316638 ns/op	84490022 B/op	  810009 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_only-32         	      16	  65974423 ns/op	84483851 B/op	  810001 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_only-32         	      18	  65539845 ns/op	84483848 B/op	  810001 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_only-32         	      16	  65633018 ns/op	84483842 B/op	  810001 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_only-32         	      18	  62905860 ns/op	84483848 B/op	  810001 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_and_iterate_sequential-32         	       1	1362166735 ns/op	106810016 B/op	 1740005 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_and_iterate_sequential-32         	       1	1351284730 ns/op	106814888 B/op	 1740007 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_and_iterate_sequential-32         	       1	1363145803 ns/op	106810128 B/op	 1740006 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_and_iterate_sequential-32         	       1	1356777445 ns/op	106815000 B/op	 1740008 allocs/op
BenchmarkNewChunkMergeIterator_ManyIterators/create_and_iterate_sequential-32         	       1	1341472321 ns/op	106815000 B/op	 1740008 allocs/op
PASS
ok  	github.com/cortexproject/cortex/pkg/querier/batch	12.455s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size/M type/performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@PaurushGarg